Add infrahubctl branch report command
#637
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
65d588a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0493f58b.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://wvd-20251114-infp388-branch.infrahub-sdk-python.pages.dev |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request transitions the project's package management and build tooling from Poetry to UV across all CI/CD workflows, development documentation, and build configuration. The Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (2)
123-160: Override working directory for uv commands in docs jobThe docs job sets
defaults.run.working-directory: ./docs, butpyproject.tomlanduv.lockexist only at the repo root. Runninguv syncanduv runfrom./docswill fail because uv cannot find the project configuration.Override the working directory for these steps to execute from the repo root:
- name: Install dependencies working-directory: . run: uv sync --all-groups --all-extras - name: "Build docs website" working-directory: . run: "uv run invoke docs"
213-266: Add explicitpython-versioninput toastral-sh/setup-uv@v4stepThe
astral-sh/setup-uvaction respects the Python version set byactions/setup-python, so the current workflow will function. However, to ensure uv creates environments for each matrix Python version, thepython-versioninput (orUV_PYTHONenvironment variable) should be explicitly set.Update the setup-uv step:
- name: Install UV uses: astral-sh/setup-uv@v4 with: version: "${{ needs.prepare-environment.outputs.UV_VERSION }}" python-version: ${{ matrix.python-version }}This makes the Python version selection explicit and ensures robust behavior across the full matrix.
🧹 Nitpick comments (6)
.gitignore (1)
11-12: Consider removing the redundant.venvpattern.Both
.venv/and.venvpatterns are present; however,.venv/alone is sufficient to ignore the virtual environment directory. The.venvpattern is technically redundant since git only tracks files within directories, not the directory itself.Consider applying this diff to remove the redundancy:
-.venv/ -.venv +.venv/Alternatively, if you prefer maximum clarity about intent, keeping both is harmless and acceptable.
AGENTS.md (1)
106-106: Optional: Consider hyphenating "on demand".Static analysis suggests "on-demand" when used as a compound adjective. This is a stylistic choice and can be deferred.
infrahub_sdk/diff.py (1)
41-52: Diff tree metadata type and Query builder align with client usage
DiffTreeDataaccurately captures the metadata and node list shape used by the newget_diff_treeclient methods, andget_diff_tree_query()builds aQuerythat:
- Filters
DiffTreebybranch,name,from_time,to_time.- Selects the expected metadata fields (
num_*,{from,to}_time,base_branch,diff_branch,name) plus the full node structure thatdiff_tree_node_to_node_diffconsumes.- Declares variables with appropriate Python types (
str,str | None,datetime | None), matching the new DateTime variable support.Only minor nit is that the GraphQL operation name
"GetDiffTree"is now used by both the older string-based query and this newQueryobject, which is allowed but can be slightly confusing when debugging; not a blocker.Also applies to: 146-206
tests/unit/ctl/test_branch_report.py (1)
12-38: Good coverage for timestamp formatting, Git diff helper, and branch report CLIThese tests thoroughly exercise:
format_timestampacross valid ISO, timezone-aware, invalid, andNoneinputs.check_git_files_changedfor files present, absent, empty responses, and HTTP errors.- The
branch reportCLI for both no proposed changes and complex proposed-change cases includingis_draft, approvals/rejections, and creator info.One small consideration: assertions like
"Is draft No"rely on exact Rich table spacing and may become fragile if column widths change; if that starts causing churn, you might relax them to focus on content rather than alignment.Also applies to: 40-126, 128-426
infrahub_sdk/ctl/branch.py (2)
26-33: Broadenformat_timestamptyping to match actual usage
format_timestampcurrently annotatestimestamp: str -> str, but it gracefully handlesNone(and tests rely onNonereturningNone). To better reflect reality and keep type-checkers happy, consider:def format_timestamp(timestamp: str | None) -> str | None: ...The implementation itself looks correct for the formats you’re handling.
189-294: Branch report flow is solid; a couple of small cleanupsThe
reportcommand nicely ties together:
- Branch metadata (creation time, status, sync_with_git, schema changes).
- Optional diff creation from
branched_fromto now (update_diffflag).- Retrieval of the diff tree and Git changes, plus a summary table of diff counts.
- Proposed changes listing with creator, draft status, approvals, and rejections.
Two small suggestions:
Git files changed handling
You initialize
git_files_changed = Noneand then immediately overwrite it with the result ofcheck_git_files_changed, so the"N/A"branch is never taken. If the intent was to show"N/A"for branches not synced with Git, you could make that explicit:git_files_changed = None if branch.sync_with_git: git_files_changed = await check_git_files_changed(client, branch=branch_name)Timestamp parsing robustness
Parsing
branch.branched_fromviadatetime.fromisoformat(branch.branched_from.replace("Z", "+00:00"))is fine for current formats, but if that field ever becomes a nativedatetime, you’ll hit anAttributeError. A small isinstance guard would make this safer, though not strictly required right now.Overall, the command behavior is clear and matches the new tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.github/workflows/ci.yml(6 hunks).github/workflows/define-versions.yml(1 hunks).github/workflows/publish-pypi.yml(1 hunks).github/workflows/release.yml(2 hunks).gitignore(1 hunks).vale/styles/spelling-exceptions.txt(3 hunks)AGENTS.md(1 hunks)CLAUDE.md(4 hunks)README.md(1 hunks)docs/AGENTS.md(1 hunks)docs/docs/python-sdk/guides/client.mdx(1 hunks)infrahub_sdk/client.py(3 hunks)infrahub_sdk/ctl/AGENTS.md(1 hunks)infrahub_sdk/ctl/branch.py(3 hunks)infrahub_sdk/ctl/cli.py(1 hunks)infrahub_sdk/diff.py(3 hunks)infrahub_sdk/graphql/constants.py(1 hunks)infrahub_sdk/graphql/renderers.py(2 hunks)infrahub_sdk/node/attribute.py(1 hunks)infrahub_sdk/node/related_node.py(2 hunks)infrahub_sdk/pytest_plugin/AGENTS.md(1 hunks)pyproject.toml(3 hunks)tasks.py(1 hunks)tests/AGENTS.md(1 hunks)tests/unit/ctl/test_branch_report.py(1 hunks)tests/unit/sdk/test_diff_summary.py(2 hunks)
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[uncategorized] ~106-~106: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Nodes load attributes and relationships on demand - Batch Operations: Support for bul...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (29)
.vale/styles/spelling-exceptions.txt (1)
23-23: LGTM!The spelling exception additions are appropriate and correctly ordered alphabetically. All four terms are legitimate:
callouts: Common documentation termDiataxis: Documentation methodology frameworkfrontmatter: Standard documentation/markdown termTyper: CLI framework used in this codebaseAlso applies to: 27-27, 47-47, 120-120
docs/docs/python-sdk/guides/client.mdx (1)
20-20: LGTM!The installation documentation correctly reflects the migration from Poetry to UV, providing users with the updated command while maintaining the pip installation option.
infrahub_sdk/ctl/cli.py (1)
7-8: LGTM!The error message has been appropriately updated to reflect the UV-based workflow while maintaining the pip installation option for standard users.
README.md (1)
59-73: LGTM!The new UV development setup section is well-documented and provides clear examples for different use cases. The commands are correct and will help developers understand how to set up their development environment with UV.
tasks.py (1)
42-44: LGTM!The documentation generation commands have been correctly updated to use UV instead of Poetry, maintaining consistency with the broader tooling migration.
Also applies to: 52-52
infrahub_sdk/node/attribute.py (1)
108-108: LGTM!Adding
updated_atto the query data structure is appropriate and aligns with the existingupdated_atattribute defined at line 53. This ensures the field is consistently available in generated queries.infrahub_sdk/graphql/renderers.py (1)
4-4: Verification complete: VARIABLE_TYPE_MAPPING supports datetime.The import of
datetimeininfrahub_sdk/graphql/renderers.py(line 4) is properly supported by theVARIABLE_TYPE_MAPPINGininfrahub_sdk/graphql/constants.py, which includes both(datetime, "DateTime!")and(datetime | None, "DateTime")mappings. The changes are correct and well-aligned.AGENTS.md (1)
1-245: Comprehensive guidance for AI coding assistants.This documentation provides excellent coverage of the repository structure, development setup, architecture, conventions, and domain-specific guidance. The structure is clear and will be valuable for AI coding assistants working with the codebase.
infrahub_sdk/pytest_plugin/AGENTS.md (1)
1-324: Excellent pytest plugin development documentation.This comprehensive guide covers the pytest plugin architecture, YAML test file format, test item classes, configuration, and common patterns. The structure is clear with practical examples that will be valuable for developers working on the custom pytest plugin.
tests/AGENTS.md (1)
1-366: Comprehensive testing guidance.This documentation provides excellent coverage of the testing framework, directory organization, running tests, pytest configuration, fixtures, and common patterns. The examples are clear and will help developers write consistent, high-quality tests.
CLAUDE.md (3)
3-5: Appropriate deprecation notice.The deprecation notice correctly directs users to the new AGENTS.md standard while maintaining backward compatibility.
15-54: UV migration commands look correct.All development commands have been properly migrated from Poetry to UV syntax, including dependency installation, formatting, linting, testing, and documentation generation.
181-181: Python version range documented.Line 181 documents CI/CD testing on Python 3.10-3.13. This should align with the Python version support documented in AGENTS.md line 19 (currently states 3.9-3.13). Please ensure consistency across all documentation.
docs/AGENTS.md (1)
1-357: Excellent documentation development guidance.This comprehensive guide covers the Docusaurus-based documentation system, Diataxis framework, directory structure, development workflow, writing guidelines, MDX components, and code quality practices. The structure and examples will be valuable for developers contributing to documentation.
infrahub_sdk/ctl/AGENTS.md (1)
1-314: Comprehensive CLI development guidance.This documentation provides excellent coverage of the
infrahubctlCLI architecture, command structure, AsyncTyper patterns, code conventions, error handling, testing, and common patterns. The practical examples and file organization details will be valuable for developers working on CLI commands..github/workflows/release.yml (3)
33-39: Proper UV setup and dependency installation.The workflow correctly migrates from Poetry to UV, using
astral-sh/setup-uv@v4anduv sync --all-groups --all-extrasfor dependency installation.
44-48: Correct version extraction using UV and tomllib.The version extraction properly uses
uv run pythonwithtomllibto read frompyproject.toml, and correctly determines prerelease/devrelease status using thepackaging.version.Versionclass. This is the appropriate approach for PEP 621-compliant projects.
58-64: Improved error messaging for tag verification.The tag version check now provides clear "Expected vs Got" output when there's a mismatch, improving debugging experience.
.github/workflows/publish-pypi.yml (1)
44-64: UV setup and dependency caching look consistentUsing
astral-sh/setup-uv@v4, caching.venvkeyed onuv.lock, and runninguv sync --all-groups --all-extrasis a good migration away from Poetry here. No issues spotted with these steps.pyproject.toml (2)
1-104: PEP 621 + hatchling migration looks coherentThe move to
[project]metadata, optional-dependencies forctl/all, dependency-groups fortests/lint/types/dev, andhatchlingas the build backend all look consistent with the new uv-based workflow. The script and pytest plugin entry points are wired correctly to existing modules.
347-352: Build backend configuration matches package layoutSpecifying
requires = ["hatchling"],build-backend = "hatchling.build", and limiting wheel packages to["infrahub_sdk"]aligns with the rest of this repo’s structure and the CI workflows usinguv build..github/workflows/ci.yml (2)
161-187: Generated-docs validation job’s uv usage is consistentFor
validate-generated-documentation, setting up Python 3.12, installing uv, runninguv sync --all-groups --all-extras, and thenuv run invoke docs-validatefrom the repo root matches the new pyproject/uv setup and should be fine.
270-302: Integration tests updated correctly to use uvSwitching the integration job to:
- install uv,
uv sync --all-groups --all-extras,uv run pytest --cov infrahub_sdk tests/integration/,uv run codecov --flags integration-tests,fits the new dependency management model and reuses the same tooling as unit tests.
infrahub_sdk/node/related_node.py (1)
67-80: Consistentupdated_atsupport for related nodesReading
updated_atfrom either the top-level data orpropertiesand always including"updated_at": Nonein_generate_query_datagives related nodes a stableupdated_atattribute (needed by consumers like the branch report). The change does alter the GraphQL selection to always include anupdated_atfield inproperties, but that’s a reasonable trade-off for consistency.Also applies to: 165-176
tests/unit/sdk/test_diff_summary.py (1)
1-8: Comprehensive tests for diff tree query and client behaviorThe additions:
- Validate the rendered structure of
get_diff_tree_query()(operation name, variables, core fields).- Exercise
get_diff_treeon both async and sync clients with full metadata, no diff, and filtered (name + time range) scenarios.- Confirm time-range validation by asserting
from_time > to_timeraisesValueError.This gives solid coverage around the new diff tree functionality and should catch regressions in both query construction and client-side handling.
Also applies to: 166-405
infrahub_sdk/ctl/branch.py (1)
35-59:check_git_files_changedlogic is soundBuilding the diff/files URL from
client.address, usingclient._getwith the client’s default timeout, and decoding JSON viadecode_jsonis consistent with the rest of the SDK. The iteration overdata[branch].values()to find any repo with a non-emptyfileslist correctly returnsTruewhen changes exist andFalseotherwise.No functional issues here.
infrahub_sdk/client.py (3)
38-38: LGTM! Import statement adds necessary dependencies.The import adds the required types and functions for the new
get_diff_treemethod. Also includesget_diff_summary_querywhich was previously used but not explicitly imported.
2582-2638: Verify query type consistency (sync version).The sync implementation has the same potential inconsistency as the async version. Line 2608 calls
query.render(), while the similarget_diff_summarymethod (line 2563) passes the query directly. Please ensure bothget_diff_tree_query()andget_diff_summary_query()are being used consistently according to their return types.
1286-1342: No issues found—the query builder pattern is correct as implemented.The apparent inconsistency between calling
.render()onget_diff_tree_query()(line 1312) and passingget_diff_summary_query()directly (line 1256) is intentional and correct. Verification confirms:
get_diff_tree_query()returns aQueryobject with a.render()method that produces a GraphQL stringget_diff_summary_query()returns a raw GraphQL string directlyexecute_graphql()expects aquery: strparameter in both cases- Each usage correctly handles its respective query builder's return type
The two query builders are designed differently—one uses a programmatic Query object pattern, the other returns a raw string—and the implementation correctly applies each pattern.
Likely an incorrect or invalid review comment.
| UV_VERSION: | ||
| value: "0.9.8" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the UV version is current and appropriate.
The workflow output has been correctly renamed from POETRY_VERSION to UV_VERSION with version "0.9.8". Please confirm that this version is appropriate and that all consuming workflows have been updated to reference UV_VERSION instead of POETRY_VERSION.
🌐 Web query:
What is the latest stable version of UV (uv package manager)?
💡 Result:
The latest stable release is uv 0.9.9 — released November 12, 2025. [1]
Sources:
[1] astral-sh/uv — Releases (GitHub).
🏁 Script executed:
#!/bin/bash
# Search for UV_VERSION and POETRY_VERSION references in workflows
echo "=== Searching for UV_VERSION references in workflows ==="
rg -n "UV_VERSION" .github/workflows/ --type yaml
echo ""
echo "=== Searching for POETRY_VERSION references in workflows ==="
rg -n "POETRY_VERSION" .github/workflows/ --type yaml
echo ""
echo "=== Searching for outputs from define-versions workflow ==="
rg -n "define-versions" .github/workflows/ --type yaml -A 3 -B 3Length of output: 2246
Update UV_VERSION to current stable release (0.9.9).
The latest stable version of UV is 0.9.9, released November 12, 2025. The current version "0.9.8" in the workflow is outdated. All consuming workflows (release.yml, publish-pypi.yml, ci.yml) correctly reference UV_VERSION, and the POETRY_VERSION reference is commented out, so the migration is complete. Update line 8 in .github/workflows/define-versions.yml to set the version to "0.9.9".
🤖 Prompt for AI Agents
In .github/workflows/define-versions.yml around lines 7 to 8, the UV_VERSION
value is set to the outdated "0.9.8"; update that line to set UV_VERSION to the
current stable release "0.9.9" so dependent workflows pick up the correct
version.
wvandeun
force-pushed
the
wvd-20251114-infp388-branch-cleanup-mechanism
branch
from
fd46ca8 to
fcbb594
Compare
github-actions
bot
added
the
type/documentation
wvandeun
force-pushed
the
wvd-20251114-infp388-branch-cleanup-mechanism
branch
from
fcbb594 to
86bc4b9
Compare
wvandeun
changed the title
infrahubctl bracnh report commandinfrahubctl branch report command
github-actions
bot
removed
the
type/documentation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
github-actions
bot
added
the
type/documentation
Replace runtime | operator with Union from typing to support Python 3.9-3.13: - graphql/constants.py: VARIABLE_TYPE_MAPPING tuples - diff.py: GraphQL query variables dict The | operator for types only works at runtime in Python 3.10+.
| (Union[bool, None], "Boolean"), | ||
| (datetime, "DateTime!"), | ||
| (datetime | None, "DateTime"), | ||
| (Union[datetime, None], "DateTime"), |
There was a problem hiding this comment.
Hey Wim, I commented on the comment that CodeRabbit added. The better approach here would be to target the infrahub-develop branch where Python 3.9 is removed. So we don't want the old style Union here as it would cause linting issues in the other branch.
Summary by CodeRabbit
Release Notes
New Features
get_diff_treemethod retrieves detailed change tracking with time-range filteringChores